Repository navigation
fix(agc): wait for the display buffer's GPU writers before the presentation lock - #7643
EagleBearerYS wants to merge 4 commits into
Conversation
…tation lock A presentation through guest memory read the display buffer inside the presenter's hold of the device lock. The flush hook of that read takes the lock again, nested, so its wait for the frame's recorded writers could not release it: every queue worker and every guest thread that needed the lock stopped for the frame's remaining GPU time on each present. The presenter now flushes the display buffer's GPU writes before it takes the lock, unless the buffer is presented from its resident image. The hook then holds the lock only at the outermost level and waits for the GPU without it. The read under the lock still flushes, so a writer recorded in between is still waited for. APS5_NO_EARLY_DISPLAY_FLUSH keeps the wait under the lock. Ported from fable-void's 531e450 onto main: main's graphicsContext() stands for his readyContext(), and the switch is added for the port.
…waits for the display buffer's writer agc_driver_present_lock_wait presents a linear display buffer through a headless surface while a long compute dispatch on queue 0x20 writes it, and takes the device lock from the test thread meanwhile. The present has to wait for the writer, and the longest lock wait beside it has to stay under a quarter of the writer's time. The pixels each present read, taken from its CPU frame dump, have to hold the value the writer stored, so the present stays ordered after its writer. A device without VK_EXT_headless_surface skips. Based on fable-void's 531e450, whose agc_driver_cpu_read_lock_waits presentation case needs his NewPathFixture and runs only on Windows.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
A Windows run, on NVIDIA: GeForce RTX 4090, driver 617.42, Windows 11 Pro 26200, WinLibs GCC 15.2.0 r7, Release,
So a Windows run of the new test still needs a GPU whose driver has AI-assisted: yes (Claude Code) |
|
@MaxLikesCode thank you for the Windows run and for checking the three failures against |
|
@EagleBearerYS a Linux run on a machine with an NVIDIA card. The new test doesn't reach the RTX 3080 here, so its numbers below are from RADV on the CPU's integrated GPU, where it separates the two orders. In 007 First Light's menu, on the RTX 3080, every present came from the resident image, where the early flush doesn't apply, so I didn't take a frame-time A/B there. Setup:
007 First Light, main menu:
Clocks on the RTX 3080 were not pinned. I measured them with
AI-assisted: yes (Claude Code) |
|
@EagleBearerYS here's Astro's Playroom (PPSA01325) on this head (870ad5d), Linux, RTX 5080 (driver 615.78.08). Each run goes into the first level with Cross pressed and takes a 10 s trace window in the same gameplay scene. Two rounds, with the order reversed in the second.
The switch was ahead in both rounds, by 5% and 9%. The GPU sat at P0 with a median graphics clock of 2812 MHz in all four runs, and the captures show the same intro, menu and gameplay with nothing missing. I didn't check which path Playroom's presents take on this card, so I can't say whether the early flush itself ran here. |
|
@SleglJan Thanks, that's the red/green GitHub's Windows runner can't give. On RADV the test passes 13 of 13 with this PR's order. With @fma-rc Thanks for the two rounds in Astro's Playroom (PPSA01325). The switch being ahead in both is worth a closer look, since the change should only move the flush out of the presentation lock. If you can run one more round of each arm with |
# Conflicts: # docs/dev/ENVIRONMENT.md
|
@fma-rc following up on my ask from 10-21: could you run one more round of each arm in Astro's Playroom (PPSA01325) with |
|
@EagleBearerYS one round per arm on head fdc4da3, Astro's Playroom on the RTX 5080,
Default: |
|
@fma-rc thanks for the profiled rounds: every present came from the resident image in both arms, so this PR's path didn't run in Astro's Playroom (PPSA01325) either. |
|
@EagleBearerYS here is a Windows run on AMD's proprietary driver, a third vendor next to the RTX 4090 and RADV runs: Radeon RX 6700 XT (gfx1031), AMD driver 32.0.21045.16010, Windows 11 26200, Ryzen 5 5600X, WinLibs GCC 15.2.0 r7, Release,
On the open question of a title whose presents go through guest memory, neither of our two titles is one. PPSA14363 (METAL GEAR SOLID Master Collection) reports |
|
@thuanquang thank you, this helps on both open points. A third driver for the new test shows that neither proprietary Windows driver has |
core/libs/prx/libSceAgcDriver/tests/execution/PresentLockWait.cpp: the two AgcDriverPresentBuffer_nid_postfix calls pass nullptr for the overlay plane parameter that boykopovar#10507 added, as main's other callers without a plane do. The merge has no textual conflict; without this the test does not build on main.
There was a problem hiding this comment.
Update: EagleBearerYS is right. This is the known Mesa 25.3.0–26.0.3 headless WSI bug (fixed in 26.0.4, see #10043), the test passes on CI's Mesa 25.2.8, and no lavapipe skip is needed. The rest is left as I first posted it.
Linux run of head 6afa8530c3 on lavapipe, a driver that has VK_EXT_headless_surface, so agc_driver_present_lock_wait doesn't skip on it. The test crashes there every time, and it crashes the same way without this PR's driver change.
Setup: Ubuntu 26.04, GCC 15.2, Release. Lavapipe from Mesa 26.0.3 (LLVM 21.1.8) and an RTX 3070 with NVIDIA 595.99.02, one at a time via VK_DRIVER_FILES. The head is merged --no-ff onto main 809c7fa2df with no conflicts. ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1, ctest -j1.
- RTX 3070: skips 3/3 (
the device has no VK_EXT_headless_surface for a presentation window), as on SleglJan's 3080. - Lavapipe:
SEGFAULTin every run, about 1.2 s in, right afterSurface capabilities drawable=64x64 … minImages=4:- this head, VVL 1.4.341: 6/6;
- this head with
APS5_NO_EARLY_DISPLAY_FLUSH=1: 3/3 with VVL 1.4.341, 2/2 with no layer; mainplus onlyPresentLockWait.cpp: 1/1;- with
MESA_VK_WSI_DEBUG=swor=linear: still crashes.
- Where: the fault is a call through a null function pointer (
rip=0). The return address is insidelibvulkan_lvp.so, at a call site that fills aVkMemoryGetFdInfoKHR(sType1000074002) just before the call. So it crashes in lavapipe's WSI while the presenter sets up the headless swapchain, before the timing part of the test runs. I have no Mesa debug symbols on this box, so I can't name the function. It may be a Mesa 26.0.3 bug; I haven't tried another Mesa version. - The rest of
^agc(minus dcc/flip/lds_atomics64) passes on both devices with the PR: 1133/1133 on the 3070, and 1132/1133 on lavapipe, where the one failure is this crash.agc_driver_flip,agc_driver_present_modeandagc_driver_presentation_passpass on lavapipe. No AGC test onmaincallsAgcDriverAttachWindow, so this test is the first one that reaches the presenter with a real surface on lavapipe.
So this run can't give a red/green for the change. The issue is that once a Linux job runs this test on lavapipe, which is what the Linux CI uses, it would probably crash rather than skip. A skip on lavapipe (VK_DRIVER_ID_MESA_LLVMPIPE) next to the headless-surface check would avoid that, unless you want to chase the Mesa side.
GPUs tested: NVIDIA GeForce RTX 3070, 595.99.02, Ubuntu 26.04 (skips); Mesa 26.0.3 lavapipe, Ubuntu 26.04 (crashes)
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
@jotitadev thank you for running it on both. The lavapipe crash is the Mesa bug from #10043's
|
|
@EagleBearerYS thanks, you're right: it's the Mesa 25.3.0–26.0.3 headless WSI bug, and the skip isn't needed. I've added a note at the top of my review. |
What
On
main, a present through guest memory reads the display buffer while the presenter holds the device lock.Driver::Presenttakes its second hold (Presentation.cpp~108), andPresentDisplayBufferreads the buffer under it: throughGpuColorTransfer.cpp~127, orReadDisplayBufferandDisplayBuffer.cpp~252. That read's flush hook takes the lock again, nested. Because the hold isn't the outermost one (Recorder.cpp~3605), the hook waits for the frame's recorded GPU writers with the lock held. Every queue worker and guest thread that needs the lock then stops for the frame's remaining GPU time, on each present.The fix is fable-void's 531e450 from his
wip/new-gpu-path, with his authorship.Ported onto
mainwith two changes, both named in the commit:graphicsContext()stands for hisreadyContext();APS5_NO_EARLY_DISPLAY_FLUSH, with itsENVIRONMENT.mdrow, keeps the old order.The test is new. His
agc_driver_cpu_read_lock_waitsneeds hisNewPathFixtureand residency layer, and runs only on Windows, soagc_driver_present_lock_waitre-implements its presentation case onmain.VK_EXT_headless_surfaceskips.Pending: a Windows run of the new test, and a title whose presents go through guest memory; none found so far. Resident Evil Village (PPSA01556) and 007 First Light (PPSA11386) at their menus, Astro's Playroom (PPSA01325) in gameplay, and, per thuanquang's report, Metal Gear Solid Master Collection (PPSA14363) and Persona 5 Royal (PPSA05110) present from the resident image (Persona 5 Royal sends at most one of about 1,300 presents through guest memory), where this PR's path doesn't run (see Tested). This stays a draft until then.
Tested
Linux, lavapipe (Mesa 25.2.8, LLVM 20.1.2), CI's configure flags, on a cloud worker,
agc_driver_present_lock_wait:--repeat until-fail:20, at most 2.42 ms over 60 attemptsPresentation.cppline revertedAPS5_NO_EARLY_DISPLAY_FLUSH=1The writer ran ~489–508 ms each time, and every present read its value.
The order check catches a misordered read. A temporary mutant removed the read's own flush (
DisplayBuffer.cpp:252). With the switch set, the test fails: "the present read 0x00000000 from the display buffer before its GPU writer stored 0xd2000001".CI's Linux mirror on
main218aa8f and on this branch:main: 1,391 passed, 69 not run, 5 failed;The 5 failures are the worker's known IPv6 tests. The only difference is the new test.
Windows, NVIDIA GeForce RTX 4090 (driver 617.42, Windows 11 Pro 26200, WinLibs GCC 15.2.0, Release), run by MaxLikesCode on this head merged into
mainad502d7 (comment):agc_driver_present_lock_waitskips 5 of 5 runs, with and withoutAPS5_NO_EARLY_DISPLAY_FLUSH=1: the NVIDIA Windows driver has noVK_EXT_headless_surface, so this GPU can't run the test;tools/select_tests.pyselects, 956 pass, 15 skip and 3 fail. The 3 fail the same way onmainon that GPU without this PR:agc_driver_bda_device(agc_driver_bda_device times out in vkWaitForFences since the BDA anchor lookup (#5859, d422c760c) #7584),agc_driver_image_table_reuse(agc_driver_image_table_reuse fails on Windows since #6346: the Windows-only CpuWriteResolutions is stale #7710) andagc_driver_pixel_lds(AGC: agc_driver_pixel_lds loses the device on its wave32 bounded draw on an RTX 4090 (Windows) #7708).tools/check_skips.py --job windowsaccepts the skips.Windows, AMD Radeon RX 6700 XT (gfx1031, driver 32.0.21045.16010, Windows 11 26200, WinLibs GCC 15.2.0 r7, Release), run by thuanquang on fdc4da3 merged onto
maine05ba6f (comment), per their report:agc_driver_present_lock_waitskips: AMD's Windows driver has noVK_EXT_headless_surfaceeither (its surface extensions areVK_KHR_surfaceandVK_KHR_win32_surface), so neither proprietary Windows driver can run the test;tools/select_tests.pyselects for this change, all pass exceptagc_host_import_cut_lifetime,agc_driver_depth_plane_partial_storeandagc_driver_sample_order_grid_reference, which fail the same way on unmodifiedmain4017310 on that GPU (3 of 3 runs in each tree), andpublic_headers, which goes over its 30 s timeout there; its audit and its 64 compile batches pass when run on their own.tools/check_skips.py --job windowsaccepts all 10 skips.Linux, RADV (the AMD Ryzen 9 7950X3D's integrated GPU, RADV RAPHAEL_MENDOCINO, Mesa 26.2.4; CachyOS, Linux 7.2.10, GCC 16.2.1, Release), run by SleglJan on this PR's 870ad5d on
mainb44d012 with the open PRs 007 First Light needs (comment):agc_driver_present_lock_waitpasses 13 of 13 runs with this PR's order; in the 8 verbose ones, the longest device-lock wait beside a present is 0.001–0.025 ms;APS5_NO_EARLY_DISPLAY_FLUSH=1it fails 5 of 5: the longest wait is 308.6–310.6 ms, behind a 347.2 ms display-buffer writer;0xd2000001in both arms. The machine's RTX 3080 (NVIDIA 615.78.08) has noVK_EXT_headless_surfaceon Linux, so the test can't run on it.007 First Light (PPSA11386), main menu, the same machine's RTX 3080, this PR's order, by SleglJan (same comment): the last
[flip]line underAPS5_PROFILE_DRAW=1reads 2,518 presents from the resident image and 0 through guest memory, so this PR's path didn't run there, and no frame-time A/B was taken.Astro's Playroom (PPSA01325), Linux, RTX 5080 (driver 615.78.08), run by fma-rc on 870ad5d (comment): frames in a 10 s gameplay window, two rounds with the order reversed in the second. This PR: 129 and 121.
APS5_NO_EARLY_DISPLAY_FLUSH=1: 136 and 132. The GPU sat at P0 at a median 2812 MHz. These counts differ by 5 and 9 % on a path that the profiled round below shows doesn't run in Playroom's gameplay, so they read as run-to-run spread, not this PR.Astro's Playroom (PPSA01325), one profiled round per arm on fdc4da3, by fma-rc (comment): the same RTX 5080,
APS5_PROFILE_DRAW=1, the last report period before the 10 s trace window closed. The last[flip]line reads 3,521 presents from the resident image by default and 3,182 withAPS5_NO_EARLY_DISPLAY_FLUSH=1, with 0 through guest memory in both (0 not pending, 0 unsuitable). So this PR's path doesn't run there either.Metal Gear Solid Master Collection (PPSA14363) and Persona 5 Royal (PPSA05110), the same RX 6700 XT, by thuanquang (same comment), per their report: Metal Gear Solid Master Collection's
[flip]lines read 0 presents through guest memory, and Persona 5 Royal sends at most one of about 1,300 that way, every other present coming from the resident image, so this PR's path doesn't run in either.Resident Evil Village (PPSA01556), Windows 11 Home 26300, NVIDIA GeForce RTX 3070 Laptop GPU (driver 617.42), AMD Ryzen 9 5900HS: this PR's 870ad5d merged with 63d5414 (fix(agc): put a retired import's unchanged ranges back in the write watch #8926's fix) on
main1ff5c8a. Each run sits at the main menu for 16 min with no input, withAPS5_LAZY_SHADER_PREPARE=1 APS5_NO_GPL=1 APS5_HOST_IMPORT_MIB=6400 APS5_TRACE_SUMMARY_EVERY=1; the other arm addsAPS5_NO_EARLY_DISPLAY_FLUSH=1. Order A B A B.APS5_NO_EARLY_DISPLAY_FLUSH=1nvidia-smi's samples (every ~0.1 s); clocks weren't locked (690–1785 MHz).APS5_PROFILE_DRAW=1), the last[flip]line reads 4,366 presents from the resident image and 19 through guest memory with this PR, and 5,007 and 4 with the switch. The guest-memory counts are already there at the first line, at 573 and 583 presents, so they're boot presents. For a resident presentPrepareDisplayReadreturns before its flush, so at the menu the arms differ only by itsPresentableResidentlookup. This pair can't show the lock wait this PR removes; it only checks Village's menu for a regression. The switch's 2–8 min lead comes from its second run: its own two runs differ by 20% there.Pending: a Windows run of the new test on a GPU whose driver has
VK_EXT_headless_surface; a title whose presents go through guest memory, none found so far.fdc4da3, a merge of
main938812d. The only conflict wasdocs/dev/ENVIRONMENT.md, wheremainaddedAPS5_NO_EMPTY_INDIRECT_SKIPnext to this PR'sAPS5_NO_EARLY_DISPLAY_FLUSH; both rows are kept, in order, and the PR's own diff is unchanged. On GitHub's runners (run), ofagc_driver_present_lock_wait,agc_driver_bda_device,agc_driver_image_table_reuseandagc_driver_pixel_lds, 3 pass 10 runs in a row on Linux andagc_driver_pixel_ldsskips; on Windows all 4 skip without a Vulkan device; none fail. The libc and libkernel suites pass on both (119 and 201 tests on Linux, 146 and 211 on Windows), run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.6afa853, a merge of
main828ac0b (run). The merge has no textual conflict, but without one change the new test doesn't build onmain: orezraey's feat(videoout): composite the overlay bus over the main output #10507 gaveAgcDriverPresentBuffer_nid_postfixaconst DisplayBuffer* planeparameter for the overlay plane. The test's two calls inPresentLockWait.cppnow passnullptrfor it, asmain's other callers without a plane do, and the merge commit's message names it. This PR's other changed lines are the same.agc_driver_present_lock_wait,agc_driver_bda_device,agc_driver_image_table_reuseandagc_driver_pixel_lds.agc_driver_present_lock_waitpassed every run; the skip isagc_driver_pixel_lds.APS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.Checklist
main218aa8f, and merges cleanly into currentmain; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)VK_EXT_headless_surface); RADV RAPHAEL_MENDOCINO (Mesa 26.2.4) and RTX 3080 (615.78.08), CachyOS, by SleglJan; RTX 5080 (615.78.08), Linux, by fma-rc; AMD Radeon RX 6700 XT (driver 32.0.21045.16010), Windows 11, by thuanquang (the new test skips there withoutVK_EXT_headless_surface); NVIDIA GeForce RTX 3070 Laptop GPU (driver 617.42), Windows 11, with Resident Evil Village